cli/compose/loader: remove some wrapper utilities and use errors.Join#6809
cli/compose/loader: remove some wrapper utilities and use errors.Join#6809thaJeztah wants to merge 2 commits intodocker:masterfrom
Conversation
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added
status/2-code-review
area/stack
kind/refactor
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Refactors the Compose config merge logic in cli/compose/loader to remove small wrapper utilities and use errors.Join for aggregating merge errors while merging multiple compose files.
Changes:
- Aggregate per-file merge errors using
errors.Joininstead of returning on the first merge failure. - Inline map-merging wrappers (volumes/networks/secrets/configs) by calling
mergo.Mapdirectly. - Fix
mergeServicesto returnnil(not the base slice) on error.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| errs = append(errs, fmt.Errorf("cannot merge configs: %w", err)) | ||
| } | ||
| if err := errors.Join(errs...); err != nil { | ||
| return nil, errors.Join(fmt.Errorf("failed to merge file %s", override.Filename), err) |
There was a problem hiding this comment.
The top-level error is built with errors.Join, which makes the file-context a separate error line and loses normal wrapping semantics. Consider wrapping the joined merge errors instead (e.g., include the filename in a single fmt.Errorf with %w), and use %q for the filename for clearer output.
| return nil, errors.Join(fmt.Errorf("failed to merge file %s", override.Filename), err) | |
| return nil, fmt.Errorf("failed to merge file %q: %w", override.Filename, err) |
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)